✨ allows ooil to escape legacy format in y*ml files inside .osparc folder
#8085
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8085 +/- ##
==========================================
+ Coverage 88.16% 89.80% +1.63%
==========================================
Files 1816 1281 -535
Lines 69980 54438 -15542
Branches 1264 255 -1009
==========================================
- Hits 61699 48888 -12811
+ Misses 7914 5470 -2444
+ Partials 367 80 -287
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
GitHK
changed the title
ooil to escape legacy formation in y*ml files inside .osparc folder
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new CLI command to escape legacy $${ sequences in YAML files under the .osparc config directory and provides corresponding tests.
- Implement
escape_dollar_braceand alegacy-escapecommand to replace$$\{with$$$$\{in.y*mlfiles. - Add unit tests for the escaping function and end-to-end tests for the new CLI command.
- Update the CLI entrypoint to register the new
legacy-escapecommand.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/service-integration/src/service_integration/cli/_escaping.py | Add escape logic and legacy_escape CLI implementation |
| packages/service-integration/src/service_integration/cli/init.py | Register legacy-escape command |
| packages/service-integration/tests/test_cli__escaping.py | Add parameterized tests for escape_dollar_brace |
| packages/service-integration/tests/test_cli.py | Add tests for the legacy-escape CLI and error helper |
Comments suppressed due to low confidence (1)
packages/service-integration/tests/test_cli.py:24
- [nitpick] The
os.EX_OKconstant may not be defined on all platforms (e.g., non-Unix). For cross-platform tests, consider asserting against literal0or guarding availability.
assert result.exit_code == os.EX_OK, _format_cli_error(result)
packages/service-integration/src/service_integration/cli/_escaping.py
Outdated
Show resolved
Hide resolved
packages/service-integration/src/service_integration/cli/_escaping.py
Outdated
Show resolved
Hide resolved
GitHK
changed the title
ooil to escape legacy formation in y*ml files inside .osparc folderooil to escape legacy format in y*ml files inside .osparc folder
sanderegg
left a comment
sanderegg
left a comment
There was a problem hiding this comment.
can you please document where this change comes from?
- I searched a bit and found there was a temporary issue with some versions of compose
- reading this PR I don't learn anything about the why, how, of this "change" if it's indeed a "change" and not a bug.
packages/service-integration/src/service_integration/cli/_escaping.py
Outdated
Show resolved
Hide resolved
I've updated the only reference I found about this |
packages/service-integration/src/service_integration/cli/__init__.py
Outdated
Show resolved
Hide resolved
sanderegg
left a comment
sanderegg
left a comment
There was a problem hiding this comment.
will not block it, but I think this would need a bit more thorough search than just making it work. what happens if they fix it? will it break again?
I'm changing it so that it can easily be revertible with an option in the publisher pipeline. |
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 415442c |
|
4 participants
What do these changes do?
With the latest versions of
dockeranddocker compose, it is no longer possible to use$$escaping in the image labels. The$$$$is now required.When the images are built, the
$$$$is converted to a single$on the labels of the images.Related issue/s
How to test
Dev-ops